Add AlchemicalArchive extraction#486
Conversation
fee79ff to
e44caaa
Compare
fbe13ff to
41b7d58
Compare
|
Awaiting new |
|
@ianmkenney would you like me to continue on this one? Otherwise happy to leave it to you! |
|
@dotsdl you can take over if you'd like |
|
Can do! Thanks for all your work on this! Will ask you for a review once I'm finished! |
Add AlchemiscaleClient.get_network_archive / get_network_archives, which bundle an AlchemicalNetwork with all successful ProtocolDAGResults for its Transformations into a gufe AlchemicalArchive, with optional user-supplied metadata. Not-found networks yield None. Implemented client-side atop the existing get_network and get_transformation_results machinery, replacing the abandoned server-side /bulk/networks/archive endpoint (which referenced undefined names and a non-existent statestore method). Also fixes a missing `json` import in client.py and restores pdr_from_bytes in utils.py (zstd import + decode fallback). Closes #246 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #486 +/- ##
==========================================
+ Coverage 80.54% 80.81% +0.27%
==========================================
Files 31 31
Lines 4877 4916 +39
==========================================
+ Hits 3928 3973 +45
+ Misses 949 943 -6 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Rework AlchemiscaleClient._get_network_archive to pair each Transformation with its results by fetching results in parallel via get_network_results and then retrieving each Transformation by its authoritative (server-side) ScopedKey. A Transformation deserialized now may not reproduce the GufeKey it had at ingestion (gufe tokenization can change across versions), so a reconstructed ScopedKey is not a reliable join and could silently drop results; the server-side ScopedKey is stable. Also broaden metadata-serialization validation to catch ValueError (e.g. circular references), and extend tests to cover string ScopedKey input, the compress=False path, and per-network metadata ordering in the bulk method. Document the GufeKey-instability gotcha in CLAUDE.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Design note: client-side archive assemblyThis PR builds Why client-side
Tradeoffs accepted
Correctness noteResults are joined to |
The docs build failed to import the package: client.py now imports gufe.archival, absent from the docs env's pinned gufe=1.3.0. Bump it to 1.10.0 to match the runtime pin. That bump surfaced a latent conf.py issue: gufe>=1.10.0 evaluates `PositiveFloat | None` at class-definition time in its settings models, which raises "unsupported operand type(s) for |" when pydantic is mocked via autodoc_mock_imports. Drop pydantic from the mock list (it is a real, installed gufe dependency); autodoc then imports gufe cleanly. Finally, render AlchemicalArchive as an inline literal in getting_started.rst rather than an intersphinx cross-reference: the gufe inventory is pinned to v1.2.0, which predates gufe.archival, so the reference cannot resolve. Verified: `sphinx -W` builds with zero warnings in both the docs.yml env and the full runtime env. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
dotsdl
left a comment
There was a problem hiding this comment.
I'm happy with this! Thanks for the initial work on this @ianmkenney!
| # had when ingested (gufe tokenization can change across versions), so | ||
| # its key is not a reliable join to the stored ScopedKey. | ||
| transformation_results = [] | ||
| for transformation_sk, pdrs in results.items(): |
There was a problem hiding this comment.
I think we should make the transformation_results ordering deterministic before constructing the AlchemicalArchive.get_network_results() builds its result dict from as_completed(...), so results.items() here reflects process completion order rather than network order or key order. That means two archive exports for the same network/results could produce different serialized archives. Could we iterate in a stable order, e.g. for transformation_sk in sorted(results, key=str): ..., or preserve the order from get_network_transformations(network)?
I think it makes sense to leave get_network_results(...) as is (since I think in general we should not keep order preserved since there could be performance reasons to keep it unsorted) and instead sort when we iterate over the results.
| - **Frozen Pydantic models**: Core models use `frozen=True` for immutability. | ||
| - **Module docstrings**: Follow the pattern `:mod:\`alchemiscale.module_name\` --- description`. | ||
| - **GUFE integration**: Core chemistry types (`AlchemicalNetwork`, `Transformation`, `ChemicalSystem`, `Protocol`) come from the `gufe` library and are stored/retrieved via their tokenization system (`GufeKey`, `GufeTokenizable`). | ||
| - **`GufeKey` is not stable across `gufe` versions**: A `ScopedKey` is stable once created (it carries the *ingestion-time* `gufe_key`), but a `GufeTokenizable` deserialized later may recompute a *different* `GufeKey` if the `gufe` tokenization has changed since ingestion. Never derive a `ScopedKey` from a freshly-deserialized object's `.key` (e.g. `ScopedKey(gufe_key=obj.key, **scope.to_dict())`) to join against stored data — such a join can silently miss instead of erroring. Instead, obtain the authoritative `ScopedKey` from the server (e.g. `get_network_transformations`, or the keys of `get_network_results`) and fetch the object *by that `ScopedKey`* (e.g. `get_transformation(sk)`). Comparisons purely among objects deserialized in the same process (e.g. "is this transformation in `network.edges`?") are safe, since both sides use the current-stack key consistently. |
There was a problem hiding this comment.
Does this just keep a running list of tips for Claude?
There was a problem hiding this comment.
This was supposed to be a question, this isn't a blocking comment.
closes #246